-
Notifications
You must be signed in to change notification settings - Fork 129
Add skill to launch CAMGI for must-gather #144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds CAMGI support to the must-gather plugin: a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script as run-camgi.sh
participant Local as okd-camgi (binary/module)
participant Container as Podman/Docker
participant CAMGI as CAMGI Service
participant Browser
User->>Script: run-camgi.sh [must-gather-path | stop]
Script->>Script: parse args & validate path/layout
alt stop
Script->>Container: locate & stop CAMGI containers (image filter)
Script-->>User: stopped / status
else start
Script->>Script: check file ownership/permissions
alt needs fix
Script->>User: prompt to fix permissions (auto-fix option)
User-->>Script: approve/deny
end
rect rgba(220,235,255,0.9)
note right of Script: Execution fallback chain
Script->>Local: attempt local binary / python -m okd_camgi
Local-->>Script: available / not found
alt not available
Script->>Container: run container (podman/docker) with mounts, port, SELinux relabel
Container->>CAMGI: start service (127.0.0.1:8080)
CAMGI-->>Script: ready
end
end
Script->>Browser: open 127.0.0.1:8080 (if possible)
Script-->>User: report URL, PID/container status, and next steps
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (6 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (6)
plugins/must-gather/README-CAMGI.md (2)
10-13: Add language identifiers to fenced code blocks.All code blocks should specify the language for proper syntax highlighting. Add
bashidentifiers to the bash code blocks.Examples:
- ``` + ```bash ./run-camgi.sh /path/to/must-gather - ``` + ```Apply this consistently to all bash blocks throughout the file (lines 10–13, 37–40, 54–56, 116–123, 136–140, 170–172).
Also applies to: 37-40, 54-56, 116-123, 136-140, 170-172
75-76: Wrap bare URLs in markdown link syntax.Several URLs are bare and should be wrapped in markdown link syntax for consistency and better rendering.
Examples:
- - CAMGI GitHub: https://github.com/elmiko/okd-camgi + - CAMGI GitHub: [https://github.com/elmiko/okd-camgi](https://github.com/elmiko/okd-camgi)Apply to lines 75–76, 106, and 160.
Also applies to: 106-106, 160-160
plugins/must-gather/commands/camgi.md (2)
47-47: Grammar: Fix verb form in file permissions description.Lines 47 and 190 should use the infinitive form "to read" instead of the noun "read".
Apply this diff:
-Must-gather files need read permissions for the container user. +Must-gather files need to read permissions for the container user.Actually, the better phrasing would be:
-Must-gather files need read permissions for the container user. +Must-gather files need read access for the container user.Or:
-Must-gather files need read permissions for the container user. +The container user needs read permissions for must-gather files.Also applies to: 190-190
58-58: Minor grammar improvements.
- Line 58: Add missing comma after closing parenthesis: "...which should not contain secrets**,**"
- Line 105: Change "local install" to "local installation" for proper noun form
Also applies to: 105-105
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (2)
35-35: Shellcheck SC2155: Separate variable declaration and assignment.Store the command output in a separate step to avoid masking return values and improve clarity.
Apply to lines 35 and 50:
- local containers=$(podman ps -q --filter ancestor=quay.io/elmiko/okd-camgi) + local containers + containers=$(podman ps -q --filter ancestor=quay.io/elmiko/okd-camgi)Also applies to: 50-50
180-180: Browser opening viaxdg-openmay silently fail in non-graphical environments.Lines 180 and 194 attempt to open the browser via
xdg-open, but this tool is not available in headless systems, SSH sessions, or container environments. The error is silently suppressed (2>/dev/null), leaving the user uncertain whether the browser opened.Consider:
- Checking if
xdg-openis available before attempting to use it- Providing clearer messaging to the user that the browser opening was skipped
- Suggesting the user manually navigate to the URL if browser opening fails
This is a UX refinement; the functionality still works (server starts regardless), but users on headless systems won't know whether the browser opening succeeded.
Also applies to: 194-194
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
PLUGINS.md(1 hunks)docs/data.json(1 hunks)plugins/must-gather/README-CAMGI.md(1 hunks)plugins/must-gather/commands/camgi.md(1 hunks)plugins/must-gather/skills/must-gather-analyzer/SKILL.md(3 hunks)plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh(1 hunks)
🧰 Additional context used
🪛 LanguageTool
plugins/must-gather/commands/camgi.md
[grammar] ~47-~47: The verb “read” needs to be in the to-infinitive form.
Context: ...File Permissions:* Must-gather files need read permissions for the container user. The...
(MISSING_TO_BEFORE_A_VERB)
[uncategorized] ~58-~58: Possible missing comma found.
Context: ... -R a+r`). This is safe for must-gather data which should not contain secrets. **Ma...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~74-~74: Consider using “inaccessible” to avoid wordiness.
Context: ...oesn't Open:** If the web interface is not accessible: - The script opens http://127.0.0.1:80...
(NOT_ABLE_PREMIUM)
[uncategorized] ~93-~93: Possible missing preposition found.
Context: ...provided as argument, ask the user - Accept either root must-gather directory or ex...
(AI_HYDRA_LEO_MISSING_TO)
[grammar] ~105-~105: The word ‘install’ is not a noun.
Context: ...erized version (podman/docker) or local install - Open browser automatically at http...
(A_INSTALL)
[grammar] ~190-~190: The verb “read” needs to be in the to-infinitive form.
Context: ...File Permissions: Must-gather files need read permissions for the container user - **...
(MISSING_TO_BEFORE_A_VERB)
🪛 markdownlint-cli2 (0.18.1)
plugins/must-gather/README-CAMGI.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
37-37: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
75-75: Bare URL used
(MD034, no-bare-urls)
76-76: Bare URL used
(MD034, no-bare-urls)
106-106: Bare URL used
(MD034, no-bare-urls)
116-116: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
136-136: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
160-160: Bare URL used
(MD034, no-bare-urls)
170-170: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh
[warning] 35-35: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 50-50: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (2)
PLUGINS.md (1)
116-116: LGTM! Documentation addition is clear, well-placed, and follows the existing format.plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (1)
151-215: Strong implementation of multiple fallback methods with verified permission checking.The
run_camgi()function demonstrates good defensive programming with clear fallback paths (local binary → Python module → podman → docker) and helpful error messages guiding users through installation.Permission checking logic (lines 116–141) verified:
- The
find ... ! -perm -004command correctly identifies files without world-read permission- The
chmod -R a+rcommand safely uses the default behavior (does not follow symlinks by default; uses-Pimplicitly)- Error suppression via
2>/dev/nullis acceptable here since the script provides fallback messaging (lines 131–133) guiding users to manually fix permissions if needed- Edge cases with special files (sockets, FIFOs) are unlikely to block most must-gather archives
The logic is sound and handles missing dependencies gracefully.
9e8da09 to
6ac0b31
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/data.json (1)
545-550: Inconsistency betweensynopsisandargument_hint(existing feedback).The
synopsisshows only[must-gather-path]butargument_hintincludes thestopoption. These should be consistent.Apply this diff to fix:
- "synopsis": "/must-gather:camgi [must-gather-path]", + "synopsis": "/must-gather:camgi [must-gather-path|stop]",
🧹 Nitpick comments (2)
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (2)
30-45: Fix Shellcheck SC2155 warning: declare and assign separately.Line 35 assigns to
containerswithin a pipeline, which masks the return value ofpodman ps. Split the declaration and assignment to properly capture the exit code.stop_camgi() { echo -e "${BLUE}Stopping camgi containers...${NC}" # Check for podman if command_exists podman; then - local containers=$(podman ps -q --filter ancestor=quay.io/elmiko/okd-camgi) + local containers + containers=$(podman ps -q --filter ancestor=quay.io/elmiko/okd-camgi) if [ -n "$containers" ]; then
48-61: Fix Shellcheck SC2155 warning: declare and assign separately (docker check).Line 50 has the same issue as line 35. Split the declaration and assignment for docker.
# Check for docker if command_exists docker; then - local containers=$(docker ps -q --filter ancestor=quay.io/elmiko/okd-camgi) + local containers + containers=$(docker ps -q --filter ancestor=quay.io/elmiko/okd-camgi) if [ -n "$containers" ]; then
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
PLUGINS.md(1 hunks)docs/data.json(1 hunks)plugins/must-gather/README-CAMGI.md(1 hunks)plugins/must-gather/commands/camgi.md(1 hunks)plugins/must-gather/skills/must-gather-analyzer/SKILL.md(3 hunks)plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- PLUGINS.md
🧰 Additional context used
🪛 LanguageTool
plugins/must-gather/commands/camgi.md
[grammar] ~47-~47: The verb “read” needs to be in the to-infinitive form.
Context: ...File Permissions:* Must-gather files need read permissions for the container user. The...
(MISSING_TO_BEFORE_A_VERB)
[uncategorized] ~58-~58: Possible missing comma found.
Context: ... -R a+r`). This is safe for must-gather data which should not contain secrets. **Ma...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~74-~74: Consider using “inaccessible” to avoid wordiness.
Context: ...oesn't Open:** If the web interface is not accessible: - The script opens http://127.0.0.1:80...
(NOT_ABLE_PREMIUM)
[uncategorized] ~93-~93: Possible missing preposition found.
Context: ...provided as argument, ask the user - Accept either root must-gather directory or ex...
(AI_HYDRA_LEO_MISSING_TO)
[grammar] ~105-~105: The word ‘install’ is not a noun.
Context: ...erized version (podman/docker) or local install - Open browser automatically at http...
(A_INSTALL)
[grammar] ~190-~190: The verb “read” needs to be in the to-infinitive form.
Context: ...File Permissions: Must-gather files need read permissions for the container user - **...
(MISSING_TO_BEFORE_A_VERB)
🪛 markdownlint-cli2 (0.18.1)
plugins/must-gather/commands/camgi.md
21-21: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
plugins/must-gather/README-CAMGI.md
21-21: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🪛 Shellcheck (0.11.0)
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh
[warning] 35-35: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 50-50: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (6)
plugins/must-gather/README-CAMGI.md (1)
1-173: Clear documentation for CAMGI integration and setup.The README provides comprehensive coverage of CAMGI features, prerequisites, troubleshooting, and technical details. The structure with Quick Start, Commands, Features, Installation, and Troubleshooting sections makes this easy to follow.
plugins/must-gather/skills/must-gather-analyzer/SKILL.md (3)
179-200: CAMGI section well-integrated into storage analysis.The new CAMGI section fits naturally after storage analysis and before the interpret-and-report phase. The documentation clearly explains when and how to use CAMGI for autoscaler investigation, with proper references to detailed docs.
261-265: CAMGI launcher script properly documented in helper scripts reference.The entry concisely documents the launcher script's purpose, output, prerequisites, and reference to detailed documentation.
293-296: New "Cluster autoscaler problems" workflow adds practical guidance.The new scenario flow (run CAMGI → inspect UI → review decisions) provides clear next steps for users troubleshooting autoscaler issues.
plugins/must-gather/commands/camgi.md (1)
1-202: Well-structured command documentation with clear implementation details.The documentation comprehensively covers prerequisites, error handling, implementation flow, and practical examples. The connection to the launcher script at
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.shis clearly documented. Users have clear guidance on permissions, port conflicts, SELinux, and browser issues.plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (1)
150-215: Well-implemented CAMGI launcher with robust fallback chain.The
run_camgi()function methodically attempts multiple approaches (local binary → Python module → containerized) with clear error messaging at each stage. The use of--webbrowserflag for local execution and automatic browser opening for containers provides good UX. Permissions checking with interactive fixing is user-friendly.
6ac0b31 to
aeeb121
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Prashanth684 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PLUGINS.md (1)
1-150: Address pipeline lint failure: Runmake updateto regenerate PLUGINS.md.The CI pipeline reports that PLUGINS.md is out of sync with plugin metadata. This must be resolved before the PR can merge.
Run
make updateto regenerate the file with the correct metadata, then commit the updated PLUGINS.md.
♻️ Duplicate comments (1)
docs/data.json (1)
669-674: Unresolved: Updatesynopsisto includestopoption.The
synopsisfield shows[must-gather-path]butargument_hintincludes thestopoption. These must be consistent.Apply this diff to align synopsis with argument_hint:
- "synopsis": "/must-gather:camgi [must-gather-path]", + "synopsis": "/must-gather:camgi [must-gather-path|stop]",Note: The pipeline also reports
docs/data.json is out of sync with plugin metadata. Runmake updateto regenerate both this file and PLUGINS.md, then verify this correction is applied.
🧹 Nitpick comments (3)
plugins/must-gather/commands/camgi.md (2)
27-27: Add language identifier to code fence.The code fence on line 27 should specify
bashas the language for proper syntax highlighting.Apply this diff:
-## Synopsis -``` -/must-gather:camgi [must-gather-path] -/must-gather:camgi stop -``` +## Synopsis +```bash +/must-gather:camgi [must-gather-path] +/must-gather:camgi stop +```
35-35: Format bare URLs according to Markdown best practices.Several bare URLs should be wrapped in inline code blocks or markdown links for consistency with linting standards (MD034).
Examples of URLs to format:
- Line 35:
https://github.com/elmiko/okd-camgi→`https://github.com/elmiko/okd-camgi`- Line 67:
http://127.0.0.1:8080→`http://127.0.0.1:8080`- Line 139, 153, 175-177: Similar formatting
This improves consistency with project markdown standards.
Also applies to: 67-67, 139-139, 153-153, 175-177
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (1)
35-35: Improve error handling: Separate variable declaration and assignment (SC2155).The current pattern masks return values from the command substitution. Declare and assign separately to properly detect command failures.
Apply this diff to both instances:
- local containers=$(podman ps -q --filter ancestor=quay.io/elmiko/okd-camgi) + local containers + containers=$(podman ps -q --filter ancestor=quay.io/elmiko/okd-camgi) if [ -n "$containers" ]; thenRepeat the same pattern for line 50 with
docker ps.This ensures that if
podman psordocker psfails, the script can detect and handle the error instead of silently continuing with an empty result.Also applies to: 50-50
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
PLUGINS.md(1 hunks)docs/data.json(1 hunks)plugins/must-gather/README-CAMGI.md(1 hunks)plugins/must-gather/commands/camgi.md(1 hunks)plugins/must-gather/skills/must-gather-analyzer/SKILL.md(3 hunks)plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/must-gather/README-CAMGI.md
🧰 Additional context used
🪛 GitHub Actions: Lint Plugins
PLUGINS.md
[error] 1-1: PLUGINS.md is out of sync with plugin metadata. Run 'make update' to update.
docs/data.json
[error] 1-1: docs/data.json is out of sync with plugin metadata. Run 'make update' to update.
🪛 LanguageTool
plugins/must-gather/commands/camgi.md
[style] ~79-~79: Consider using “inaccessible” to avoid wordiness.
Context: ...oesn't Open:** If the web interface is not accessible: - The script opens http://127.0.0.1:80...
(NOT_ABLE_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
plugins/must-gather/commands/camgi.md
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
35-35: Bare URL used
(MD034, no-bare-urls)
67-67: Bare URL used
(MD034, no-bare-urls)
139-139: Bare URL used
(MD034, no-bare-urls)
153-153: Bare URL used
(MD034, no-bare-urls)
175-175: Bare URL used
(MD034, no-bare-urls)
176-176: Bare URL used
(MD034, no-bare-urls)
177-177: Bare URL used
(MD034, no-bare-urls)
🪛 Shellcheck (0.11.0)
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh
[warning] 35-35: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 50-50: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (3)
plugins/must-gather/skills/must-gather-analyzer/SKILL.md (1)
192-213: LGTM! CAMGI documentation well integrated.The CAMGI sections are properly documented with usage instructions, helper script reference, and workflow examples. The integration with existing analysis guidance is clear.
Also applies to: 274-278, 306-309
plugins/must-gather/commands/camgi.md (1)
2-2: Description appropriately scopes CAMGI's multi-purpose use.The description and detailed capabilities list (lines 20–28) properly convey CAMGI's broader applications beyond autoscaler analysis, including cluster operators, Machines, MachineSets, and infrastructure inspection, addressing the past observation about CAMGI's versatility.
Also applies to: 15-29
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (1)
1-241: LGTM! Well-structured launcher script with good error handling and fallback strategies.The script demonstrates solid shell scripting practices:
- Multiple methods for running CAMGI (local binary → Python module → containers)
- Intelligent must-gather directory auto-detection (lines 97–109)
- Permission checking with user-interactive fixes (lines 112–141)
- Proper use of color output for UX
- SELinux-aware volume mounting with
:Zflag (lines 183, 196)- Clear error messages with remediation guidance (lines 200–214)
The only improvement needed is addressing the SC2155 shellcheck warnings noted in the separate comment above.
Please confirm that the script has been tested with both podman and docker, and that the browser launching with
xdg-openworks in your target environments.
aeeb121 to
eab8848
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/data.json (1)
669-674: Inconsistency betweensynopsisandargument_hint(duplicate issue).The
synopsisshows only[must-gather-path]butargument_hintincludes both[must-gather-path|stop]. For consistency and clarity, thesynopsisshould reflect both available options.Apply this diff:
- "synopsis": "/must-gather:camgi [must-gather-path]", + "synopsis": "/must-gather:camgi [must-gather-path|stop]",
🧹 Nitpick comments (1)
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (1)
35-35: Fix shellcheck SC2155 violations: declare and assign separately.Combining
localdeclaration with command substitution masks return values frompodmananddockercommands. If either command fails, the error is silently hidden.Apply this diff to fix both occurrences:
- stop_camgi() { - echo -e "${BLUE}Stopping camgi containers...${NC}" - - # Check for podman - if command_exists podman; then - local containers=$(podman ps -q --filter ancestor=quay.io/elmiko/okd-camgi) + local containers + containers=$(podman ps -q --filter ancestor=quay.io/elmiko/okd-camgi)Apply the same pattern at line 50 for the
dockercommand.Also applies to: 50-50
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
PLUGINS.md(1 hunks)docs/data.json(1 hunks)plugins/must-gather/README-CAMGI.md(1 hunks)plugins/must-gather/commands/camgi.md(1 hunks)plugins/must-gather/skills/must-gather-analyzer/SKILL.md(3 hunks)plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- PLUGINS.md
🧰 Additional context used
🪛 LanguageTool
plugins/must-gather/commands/camgi.md
[style] ~79-~79: Consider using “inaccessible” to avoid wordiness.
Context: ...oesn't Open:** If the web interface is not accessible: - The script opens http://127.0.0.1:80...
(NOT_ABLE_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
plugins/must-gather/skills/must-gather-analyzer/SKILL.md
216-216: Bare URL used
(MD034, no-bare-urls)
281-281: Bare URL used
(MD034, no-bare-urls)
plugins/must-gather/commands/camgi.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
80-80: Bare URL used
(MD034, no-bare-urls)
81-81: Bare URL used
(MD034, no-bare-urls)
111-111: Bare URL used
(MD034, no-bare-urls)
121-121: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
146-146: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
170-170: Bare URL used
(MD034, no-bare-urls)
180-180: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
186-186: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
192-192: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
208-208: Bare URL used
(MD034, no-bare-urls)
209-209: Bare URL used
(MD034, no-bare-urls)
plugins/must-gather/README-CAMGI.md
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
35-35: Bare URL used
(MD034, no-bare-urls)
67-67: Bare URL used
(MD034, no-bare-urls)
139-139: Bare URL used
(MD034, no-bare-urls)
153-153: Bare URL used
(MD034, no-bare-urls)
175-175: Bare URL used
(MD034, no-bare-urls)
176-176: Bare URL used
(MD034, no-bare-urls)
177-177: Bare URL used
(MD034, no-bare-urls)
🪛 Shellcheck (0.11.0)
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh
[warning] 35-35: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 50-50: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (2)
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (1)
1-241: Script implementation looks solid overall.The error handling, user guidance, and fallback strategy are well-designed. The auto-detection of must-gather subdirectory structure (lines 98–109) and interactive permission fixing (lines 112–141) provide good user experience. Helpful error messages and multiple launch methods make the script robust.
Once the SC2155 warnings are addressed, this is ready.
plugins/must-gather/skills/must-gather-analyzer/SKILL.md (1)
192-217: CAMGI documentation additions are well-integrated.The CAMGI section (lines 192–217) provides comprehensive usage details, the helper scripts reference (lines 279–284) is properly formatted, and the new scenario (lines 312–316) fits naturally into the existing structure. The duplicate section from the previous review has been resolved.
Also applies to: 279-284, 312-316
eab8848 to
7e45466
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
♻️ Duplicate comments (1)
docs/data.json (1)
669-674: The synopsis is still inconsistent with argument_hint; include thestopoption.The
synopsisshows only[must-gather-path]butargument_hintincludes thestopoption. Based on the command documentation and usage, the synopsis should reflect both available options.Apply this diff to fix:
- "synopsis": "/must-gather:camgi [must-gather-path]", + "synopsis": "/must-gather:camgi [must-gather-path|stop]",
🧹 Nitpick comments (1)
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (1)
30-64: Duplicated container stop logic across podman and docker checks.The
stop_camgi()function repeats the same logic for podman and docker. Consider refactoring to reduce duplication:stop_camgi_runtime() { local runtime="$1" local containers=$($runtime ps -q --filter ancestor=quay.io/elmiko/okd-camgi 2>/dev/null) if [ -n "$containers" ]; then echo "$containers" | while read container; do echo -e "${BLUE}Stopping container: $container${NC}" $runtime stop "$container" done echo -e "${GREEN}Stopped camgi container(s)${NC}" fi } # Then in stop_camgi(): for runtime in podman docker; do if command_exists "$runtime"; then stop_camgi_runtime "$runtime" fi doneThis reduces the ~35 lines of duplication to a single loop. However, the current implementation is functional and the duplication may be acceptable for maintainability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
PLUGINS.md(1 hunks)docs/data.json(1 hunks)plugins/must-gather/README-CAMGI.md(1 hunks)plugins/must-gather/commands/camgi.md(1 hunks)plugins/must-gather/skills/must-gather-analyzer/SKILL.md(3 hunks)plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/must-gather/README-CAMGI.md
- PLUGINS.md
🧰 Additional context used
🪛 LanguageTool
plugins/must-gather/commands/camgi.md
[style] ~79-~79: Consider using “inaccessible” to avoid wordiness.
Context: ...oesn't Open:** If the web interface is not accessible: - The script opens http://127.0.0.1:80...
(NOT_ABLE_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
plugins/must-gather/commands/camgi.md
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
35-35: Bare URL used
(MD034, no-bare-urls)
67-67: Bare URL used
(MD034, no-bare-urls)
139-139: Bare URL used
(MD034, no-bare-urls)
153-153: Bare URL used
(MD034, no-bare-urls)
175-175: Bare URL used
(MD034, no-bare-urls)
176-176: Bare URL used
(MD034, no-bare-urls)
177-177: Bare URL used
(MD034, no-bare-urls)
plugins/must-gather/skills/must-gather-analyzer/SKILL.md
216-216: Bare URL used
(MD034, no-bare-urls)
281-281: Bare URL used
(MD034, no-bare-urls)
🪛 Shellcheck (0.11.0)
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh
[warning] 35-35: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 50-50: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (6)
plugins/must-gather/skills/must-gather-analyzer/SKILL.md (1)
312-316: Good addition to common scenarios.The new scenario section for cluster infrastructure/autoscaler problems provides clear guidance for users encountering these issues. The workflow is consistent with other scenarios and appropriately references the CAMGI tool.
plugins/must-gather/commands/camgi.md (1)
1-216: Comprehensive and well-structured command documentation.The documentation provides excellent coverage of CAMGI functionality, prerequisites, error handling, implementation details, and usage examples. The progressive disclosure of information (from simple usage to technical details) follows good documentation practices. The error handling section is particularly valuable for user experience.
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh (4)
173-198: Browser auto-launch may fail silently on some systems; ensure fallback is clear.The script attempts to launch the browser using
xdg-openin the background at lines 180 and 194. While the fallback messaging is good (lines 175-176),xdg-openmay not exist or work on all systems (e.g., headless servers, non-Linux). The silent failure (with2>/dev/null) is appropriate, but consider adding a note in output about manual URL access.Current behavior is acceptable given the existing fallback guidance, but document this design decision.
151-215: Well-structured multi-method startup strategy with good error handling.The
run_camgi()function implements a sensible priority order:
- Local binary (fastest)
- Python module (good fallback)
- Containerized (works when local not available)
Each method includes appropriate feedback, and the final error message is helpful with actionable install instructions. The integration with slash commands via documented scripts is appropriate.
92-109: Robust must-gather path detection with helpful fallback.The path validation includes auto-detection of the subdirectory structure (lines 98-109), which is user-friendly. The search pattern for
*sha256*is appropriate for OpenShift must-gather format. Good defensive programming.
111-141: Interactive permission fixing is user-friendly and safe.The permission check at lines 111-141 is well-implemented:
- Detects restrictive permissions
- Prompts user interactively
- Provides fallback if user declines
- Clear guidance on manual fix
One note: The
findcommand at line 116 with! -perm -004checks for files without world-read. For must-gather data (which should not contain secrets), this is appropriate. Good defensive coding.
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh
Outdated
Show resolved
Hide resolved
|
@Prashanth684 do we want to address the coderabbit review? Also, does this make more sense as a command? Why have you chosen to package this as a skill? |
7e45466 to
59656c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/data.json (1)
669-674: Inconsistentsynopsisandargument_hintfor camgi command.The
synopsisshould reflect all available options. It currently only shows[must-gather-path], butargument_hintincludes thestopoption. Update the synopsis for consistency:{ "name": "camgi", "description": "Launch CAMGI (Cluster Autoscaler Must-Gather Inspector) web interface to analyze cluster infrastructure and autoscaler behavior", - "synopsis": "/must-gather:camgi [must-gather-path]", + "synopsis": "/must-gather:camgi [must-gather-path|stop]", "argument_hint": "[must-gather-path|stop]" }
🧹 Nitpick comments (6)
plugins/must-gather/commands/camgi.md (6)
10-13: Add language specifier to synopsis code block (line 10).Markdown linting requires language specifiers for fenced code blocks. The synopsis block should use
bashsince it shows shell command syntax:## Synopsis - ``` + ```bash /must-gather:camgi [must-gather-path] /must-gather:camgi stop - ``` + ```
42-48: Add language specifier to must-gather directory structure block (line 42).Add
textas the language specifier for the directory listing:Must-gather data with autoscaler information: - ``` + ```text must-gather/ └── registry-ci-openshift-org-origin-...-sha256-<hash>/ ├── cluster-scoped-resources/ ├── namespaces/ └── ... - ``` + ```
59-61: Add language specifier to permission fix command block (line 59).If container shows permission denied errors, the script will prompt: - ``` + ```bash Fix permissions now? (Y/n) - ``` + ```
121-143: Add language specifier to success output example block (line 121).The output block should use
textas the language specifier:**On Successful Launch:** - ``` + ```text CAMGI is now running! The web interface should open automatically in your browser at: http://127.0.0.1:8080 ... (rest of output) - ``` + ```
146-149: Add language specifier to stop output example block (line 146).**On Stop:** - ``` + ```text Stopping all CAMGI containers... CAMGI stopped successfully. - ``` + ```
179-195: Add language specifiers to example command blocks (lines 180, 186, 192).The example blocks showing command invocations should include
bashas the language specifier:1. **Start CAMGI with path**: - ``` + ```bash /must-gather:camgi /home/user/must-gatherLaunches CAMGI with the provided must-gather path and opens browser.
- Start CAMGI interactively:
/must-gather:camgiAsks for must-gather path, then launches CAMGI.
- Stop CAMGI:
Stops all running CAMGI containers./must-gather:camgi stop</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Cache: Disabled due to data retention organization setting** **Knowledge base: Disabled due to data retention organization setting** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 7e454666710729a6977e02ff44f0031cb37b3fc0 and 59656c07acd65254c84d0b55c38dab8fcd28b58a. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `PLUGINS.md` (1 hunks) * `docs/data.json` (1 hunks) * `plugins/must-gather/README-CAMGI.md` (1 hunks) * `plugins/must-gather/commands/camgi.md` (1 hunks) * `plugins/must-gather/skills/must-gather-analyzer/SKILL.md` (3 hunks) * `plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary> * plugins/must-gather/README-CAMGI.md * PLUGINS.md </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>plugins/must-gather/commands/camgi.md</summary> [style] ~79-~79: Consider using “inaccessible” to avoid wordiness. Context: ...oesn't Open:** If the web interface is not accessible: - The script opens [http://127.0.0.1:8... (NOT_ABLE_PREMIUM) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>plugins/must-gather/commands/camgi.md</summary> 10-10: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 42-42: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 59-59: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 121-121: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 146-146: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 170-170: Bare URL used (MD034, no-bare-urls) --- 180-180: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 186-186: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 192-192: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 Shellcheck (0.11.0)</summary> <details> <summary>plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh</summary> [error] 36-36: Remove spaces around = to assign (or use [ ] to compare, or quote '=' if literal). (SC2283) --- [warning] 36-36: Quote this to prevent word splitting. (SC2046) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>plugins/must-gather/skills/must-gather-analyzer/SKILL.md (1)</summary><blockquote> `192-217`: **✓ CAMGI documentation well integrated.** The CAMGI section is well-documented with proper markdown formatting, wrapped URLs, and clear examples. The integration with other analysis methods is seamless. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh
Outdated
Show resolved
Hide resolved
59656c0 to
ad64764
Compare
2d6015f to
dcfd27e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
PLUGINS.md(1 hunks)docs/data.json(1 hunks)plugins/must-gather/README-CAMGI.md(1 hunks)plugins/must-gather/commands/camgi.md(1 hunks)plugins/must-gather/skills/must-gather-analyzer/SKILL.md(3 hunks)plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- PLUGINS.md
- plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh
- docs/data.json
🧰 Additional context used
🪛 LanguageTool
plugins/must-gather/commands/camgi.md
[style] ~79-~79: Consider using “inaccessible” to avoid wordiness.
Context: ...oesn't Open:** If the web interface is not accessible: - The script opens [http://127.0.0.1:8...
(NOT_ABLE_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
plugins/must-gather/README-CAMGI.md
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
35-35: Bare URL used
(MD034, no-bare-urls)
67-67: Bare URL used
(MD034, no-bare-urls)
139-139: Bare URL used
(MD034, no-bare-urls)
153-153: Bare URL used
(MD034, no-bare-urls)
175-175: Bare URL used
(MD034, no-bare-urls)
176-176: Bare URL used
(MD034, no-bare-urls)
177-177: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (1)
plugins/must-gather/skills/must-gather-analyzer/SKILL.md (1)
192-217: Unable to verify CAMGI duplication due to repository access issues.The verification scripts cannot execute because the repository is not accessible in the current environment. To complete this verification, I would need to:
- Count occurrences of
#### CAMGIheaders in the full SKILL.md file- Confirm whether the file contains exactly one CAMGI section
- Verify that lines 201-222 (where the duplicate was reportedly located) no longer contain duplicate content
Please manually verify by running:
grep -n "#### CAMGI" plugins/must-gather/skills/must-gather-analyzer/SKILL.mdThis will show all CAMGI section locations. There should be exactly one match if the duplicate has been removed.
plugins/must-gather/README-CAMGI.md
Outdated
| 1. Detect must-gather subdirectory structure | ||
| 2. **Check and fix file permissions** (with your confirmation) | ||
| 3. Start camgi (containerized or local install) | ||
| 4. Open web interface at http://127.0.0.1:8080 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap bare URLs in markdown link syntax.
Per markdownlint (MD034), bare URLs must be wrapped in markdown link syntax. Fix these instances:
- 4. Open web interface at http://127.0.0.1:8080
+ 4. Open web interface at [http://127.0.0.1:8080](http://127.0.0.1:8080)
- **Automatic Browser Opening**
- Opens http://127.0.0.1:8080 automatically
+ **Automatic Browser Opening**
+ Opens [http://127.0.0.1:8080](http://127.0.0.1:8080) automaticallyAlso wrap these URLs (lines 139, 153, 175–177):
- Line 139:
http://127.0.0.1:8080→[http://127.0.0.1:8080](http://127.0.0.1:8080) - Line 153:
http://127.0.0.1:9090→[http://127.0.0.1:9090](http://127.0.0.1:9090) - Line 175:
https://github.com/elmiko/okd-camgi→[https://github.com/elmiko/okd-camgi](https://github.com/elmiko/okd-camgi) - Line 176:
https://pypi.org/project/okd-camgi/→[https://pypi.org/project/okd-camgi/](https://pypi.org/project/okd-camgi/) - Line 177:
https://docs.openshift.com/...→[https://docs.openshift.com/...](https://docs.openshift.com/...)
Also applies to: 67-67, 139-139, 153-153, 175-177
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
35-35: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
plugins/must-gather/README-CAMGI.md lines 35, 67, 139, 153, 175-177: several
bare URLs violate markdownlint MD034; wrap each bare URL in markdown link syntax
(e.g. change http://127.0.0.1:8080 to
[http://127.0.0.1:8080](http://127.0.0.1:8080), http://127.0.0.1:9090 to
[http://127.0.0.1:9090](http://127.0.0.1:9090),
https://github.com/elmiko/okd-camgi to
[https://github.com/elmiko/okd-camgi](https://github.com/elmiko/okd-camgi),
https://pypi.org/project/okd-camgi/ to
[https://pypi.org/project/okd-camgi/](https://pypi.org/project/okd-camgi/), and
the OpenShift docs URL to its equivalent [URL](URL) form) so all bare links are
enclosed in [text](url) markdown.
split the camgi part out of openshift-eng#133 to make it easier to review and more readable
dcfd27e to
2af3479
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/data.json (1)
688-693: I'm unable to access the repository directly due to cloning failures in the sandbox environment. However, I can verify the review comment based on the evidence provided within it.The review comment includes the actual code snippet showing:
- Lines 688-693: The camgi command entry with
argument_hint:"[must-gather-path|stop]"andsynopsis:"/must-gather:camgi [must-gather-path]"The inconsistency is clear and verifiable from the provided snippet itself:
- The
argument_hintexplicitly includes two options:must-gather-pathandstop- The
synopsisonly showsmust-gather-path, omitting thestopoptionThis is a legitimate inconsistency. When a command supports multiple argument options (indicated by the pipe
|inargument_hint), both should be reflected in the synopsis for accurate documentation.The review comment is well-founded, the suggested fix is appropriate, and the concern about this being previously flagged but not fixed is valid.
The
synopsisfield for the camgi command must include thestopoption to matchargument_hint.The current inconsistency shows
argument_hintincludes[must-gather-path|stop]butsynopsisonly shows[must-gather-path]. Apply the suggested fix:{ "argument_hint": "[must-gather-path|stop]", "description": "Launch CAMGI (Cluster Autoscaler Must-Gather Inspector) web interface to analyze cluster infrastructure and autoscaler behavior", "name": "camgi", - "synopsis": "/must-gather:camgi [must-gather-path]" + "synopsis": "/must-gather:camgi [must-gather-path|stop]" }
🧹 Nitpick comments (2)
plugins/must-gather/commands/camgi.md (1)
10-13: Add language specifiers to fenced code blocks (markdown MD040).Several code blocks lack language specifiers required by markdown linting. Apply these fixes:
## Synopsis -``` +```bash /must-gather:camgi [must-gather-path] /must-gather:camgi stop -``` +```bashMust-gather data with autoscaler information: -``` +```text must-gather/ └── registry-ci-openshift-org-origin-...-sha256-<hash>/ ├── cluster-scoped-resources/ ├── namespaces/ └── ... -``` +```text**Manual fix** (if needed): -```bash +```bash chmod -R a+r /path/to/must-gather -``` +```bashFor the output/message blocks (lines 121–143, 146–149), use
text:**On Successful Launch:** -``` +```text CAMGI is now running! ... -``` +```text**On Stop:** -``` +```text Stopping all CAMGI containers... CAMGI stopped successfully. -``` +```textSimilarly, update example command blocks (lines 180–195) to use
bash:1. **Start CAMGI with path**: - ``` + ```bash /must-gather:camgi /home/user/must-gather - ``` + ```bash2. **Start CAMGI interactively**: - ``` + ```bash /must-gather:camgi - ``` + ```bash3. **Stop CAMGI**: - ``` + ```bash /must-gather:camgi stop - ``` + ```bashAlso applies to: 42-48, 59-62, 121-143, 146-149, 180-195
plugins/must-gather/README-CAMGI.md (1)
27-29: Fix markdown linting issues: add language specifier and wrap bare URLs.Apply these fixes for markdown compliance:
- Add
bashlanguage specifier to code block (line 27):Or use the slash command from anywhere: -``` +```bash /must-gather:camgi /path/to/must-gather -``` +```bash
- Wrap bare URLs in markdown link syntax (lines 139, 153, 175–177):
**Solution:** Use http://127.0.0.1:8080 instead of localhost + **Solution:** Use [http://127.0.0.1:8080](http://127.0.0.1:8080) instead of localhostThen access at http://127.0.0.1:9090 + Then access at [http://127.0.0.1:9090](http://127.0.0.1:9090)## References -- CAMGI GitHub: https://github.com/elmiko/okd-camgi -- CAMGI on PyPI: https://pypi.org/project/okd-camgi/ -- Must-Gather Documentation: https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html +- CAMGI GitHub: [https://github.com/elmiko/okd-camgi](https://github.com/elmiko/okd-camgi) +- CAMGI on PyPI: [https://pypi.org/project/okd-camgi/](https://pypi.org/project/okd-camgi/) +- Must-Gather Documentation: [https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html](https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html)Also applies to: 139-139, 153-153, 175-177
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
PLUGINS.md(1 hunks)docs/data.json(1 hunks)plugins/must-gather/README-CAMGI.md(1 hunks)plugins/must-gather/commands/camgi.md(1 hunks)plugins/must-gather/skills/must-gather-analyzer/SKILL.md(3 hunks)plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/must-gather/skills/must-gather-analyzer/SKILL.md
- plugins/must-gather/skills/must-gather-analyzer/scripts/run-camgi.sh
- PLUGINS.md
🧰 Additional context used
🪛 LanguageTool
plugins/must-gather/commands/camgi.md
[style] ~79-~79: Consider using “inaccessible” to avoid wordiness.
Context: ...oesn't Open:** If the web interface is not accessible: - The script opens [http://127.0.0.1:8...
(NOT_ABLE_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
plugins/must-gather/README-CAMGI.md
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
139-139: Bare URL used
(MD034, no-bare-urls)
153-153: Bare URL used
(MD034, no-bare-urls)
175-175: Bare URL used
(MD034, no-bare-urls)
176-176: Bare URL used
(MD034, no-bare-urls)
177-177: Bare URL used
(MD034, no-bare-urls)
plugins/must-gather/commands/camgi.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
121-121: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
146-146: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
180-180: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
186-186: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
192-192: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
addressed the comments. The reason for making it a skill was to be more prescriptive and deterministic when deploying camgi, i.e consistently use the same image - deploy the containers in the same fashion rather than leaving it up to the LLM to decide. |
split the camgi part out of #133 to make it easier to review and more readable
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.